libvchan: Fix handling of invalid ring buffer indices
authorMarek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Thu, 6 Feb 2014 15:44:41 +0000 (16:44 +0100)
committerJan Beulich <jbeulich@suse.com>
Thu, 6 Feb 2014 15:44:41 +0000 (16:44 +0100)
commit2efcb0193bf3916c8ce34882e845f5ceb1e511f7
tree58c6953d843b39f80f6c1d1cc4f1f0bb4c639568
parent2e1cba2da4631c5cd7218a8f30d521dce0f41370
libvchan: Fix handling of invalid ring buffer indices

The remote (hostile) process can set ring buffer indices to any value
at any time. If that happens, it is possible to get "buffer space"
(either for writing data, or ready for reading) negative or greater
than buffer size.  This will end up with buffer overflow in the second
memcpy inside of do_send/do_recv.

Fix this by introducing new available bytes accessor functions
raw_get_data_ready and raw_get_buffer_space which are robust against
mad ring states, and only return sanitised values.

Proof sketch of correctness:

Now {rd,wr}_{cons,prod} are only ever used in the raw available bytes
functions, and in do_send and do_recv.

The raw available bytes functions do unsigned arithmetic on the
returned values.  If the result is "negative" or too big it will be
>ring_size (since we used unsigned arithmetic).  Otherwise the result
is a positive in-range value representing a reasonable ring state, in
which case we can safely convert it to int (as the rest of the code
expects).

do_send and do_recv immediately mask the ring index value with the
ring size.  The result is always going to be plausible.  If the ring
state has become mad, the worst case is that our behaviour is
inconsistent with the peer's ring pointer.  I.e. we read or write to
arguably-incorrect parts of the ring - but always parts of the ring.
And of course if a peer misoperates the ring they can achieve this
effect anyway.

So the security problem is fixed.

This is XSA-86.

(The patch is essentially Ian Jackson's work, although parts of the
commit message are by Marek.)

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
tools/libvchan/io.c